Add dialyxir 1.1.0 support #583
Conversation
============= This adds a Dialyzer check to CI and cleans up existing warnings.
hazen
force-pushed
the
add-dialyxir-1.1.0-support
branch
from
8cacc2a
to
0807cdb
Compare
| - run: mix do deps.get | ||
| - save_cache: | ||
| key: mix-cache-{{ checksum "mix.lock" }} | ||
| paths: "deps" | ||
|
|
||
| - run: mix format --check-formatted | ||
| - run: mix test | ||
| - run: mix dialyzer | ||
| - save_cache: | ||
| key: dialyzer-cache-21.2-1.8.1 |
There was a problem hiding this comment.
Do you know if it's possible to use the checksum here so that the cache is busted when the dependencies change? That way we don't have to manually update this when we update dependencies.
We could use the ".tool-versions" file, since that's the source of truth for Erlang and Elixir versions. So if that file changes, it means we should bust this cache.
What do you think? Would it be possible to do this?
- save_cache:
key: dialyzer-cache-{{ checksum ".tool-versions" }}
And the corresponding change in the -restore_cache entry?
| @@ -0,0 +1,3 @@ | |||
| [ | |||
| {"lib/bamboo/formatter.ex", :callback_type_mismatch, 73} | |||
There was a problem hiding this comment.
Do you know what this warning was about and why we're ignoring it?
| def raise_api_error(message), do: raise(__MODULE__, message: message) | ||
|
|
||
| @spec raise_api_error(String.t(), any(), Keyword.t()) :: no_return() |
There was a problem hiding this comment.
I'm not 100% sure that the third argument will always be a Keyword.t(). I imagine it can also be a map. Could we relax that to an Enum.t(), since that's a more relaxed version that includes both keyword lists and maps?
| @spec raise_api_error(String.t(), any(), Keyword.t()) :: no_return() | |
| @spec raise_api_error(String.t(), any(), Enum.t()) :: no_return() |
| @@ -142,6 +142,7 @@ defmodule Bamboo.Mailer do | |||
| Email.welcome_email | |||
| |> Mailer.deliver_now(config: %{username: "Emma", smtp_port: 2525}) | |||
| """ | |||
| @spec deliver_now(any()) :: no_return() | |||
There was a problem hiding this comment.
What do you think about changing the any() to Email.t() in all of these @spec definition changes? I think the first argument is supposed to be an email (even though I realize these functions only raise an error).
germsvel
added
the
waiting-for-info
|
Closing this as we're incorporating Dialyzer in #668 |
When trying to integrate bamboo 2.0.0, I encountered a Dialyzer error. Upon further inspection, it does not look like Dialyzer is part of the build infrastructure, so this an attempt to integrate dialyxir with the project and with CI.